Skip to content

Fix cargo-gpu in build script failing when called by Miri or Clippy #335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 22, 2025

Conversation

tuguzT
Copy link
Contributor

@tuguzT tuguzT commented Jul 16, 2025

cargo-gpu Rust-GPU/cargo-gpu#101

@Firestar99: I've added a new CargoCmd struct that should handle all env vars in one central place for both spirv-builder and cargo-gpu install. In case of a failure, it also debug! logs the env vars which were removed and which were inherited with their values, to make debugging these sort of issues easier in the future.

As stated in Rust-GPU/cargo-gpu#93, build script which uses rustc_codegen_spirv via cargo-gpu failed to build shader crate while using Miri.
This pull request fixes this issue by removing RUSTC_WRAPPER environment variable from cargo call.

@tuguzT tuguzT changed the title Make build succeed when called by Miri Make shader crate build succeed when called by Miri Jul 16, 2025
@Firestar99 Firestar99 changed the title Make shader crate build succeed when called by Miri Make shader crate build succeed when called by Miri or Clippy Jul 17, 2025
@Firestar99 Firestar99 changed the title Make shader crate build succeed when called by Miri or Clippy Fix cargo-gpu in build script failing when called by Miri or Clippy Jul 17, 2025
@tuguzT tuguzT marked this pull request as ready for review July 17, 2025 21:51
@tuguzT
Copy link
Contributor Author

tuguzT commented Jul 17, 2025

Please look at Rust-GPU/cargo-gpu#101 for additional context

Copy link
Collaborator

@LegNeato LegNeato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! 🍻

Looks good, just one small thing.

// overwritten by spirv-builder anyway
cargo.env_remove("CARGO_ENCODED_RUSTFLAGS");

cargo
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seem to be added with no comment, and RUSTC_WRAPPER is both here and above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just removed duplicates from removed env vars and tried to add relevant comments

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh my bad, thanks for fixing it

@Firestar99
Copy link
Member

Sorry for the constant canceling of CI, see #342

I'm waiting for #339 to complete and then I can retrigger this one

@Firestar99 Firestar99 enabled auto-merge July 21, 2025 11:05
@tuguzT tuguzT requested a review from LegNeato July 22, 2025 08:01
Copy link
Collaborator

@LegNeato LegNeato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for the PR 🚀

@Firestar99 Firestar99 added this pull request to the merge queue Jul 22, 2025
Merged via the queue into Rust-GPU:main with commit 3e89e6c Jul 22, 2025
33 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants